ci: scope permissions to jobs and fix workflow syntax#4066
ci: scope permissions to jobs and fix workflow syntax#4066PatStLouis wants to merge 2 commits intoopenwallet-foundation:mainfrom
Conversation
Signed-off-by: Patrick St-Louis <patrick.st-louis@opsecid.ca>
|
|
Looks good to me. Only thing I'm a bit concerned about is some of the workflows don't actually run unless there is source code changes so I'm unsure if they will be successful or not. One way to test is to change the target repo and do a source code change in your fork. Or we can merge it and try some manual runs. |
There was a problem hiding this comment.
Pull request overview
This PR tightens GitHub Actions security by scoping GITHUB_TOKEN permissions to individual jobs (instead of workflow-wide defaults) and fixes a couple of workflow syntax details, aiming to keep behavior unchanged while reducing the permission footprint.
Changes:
- Move workflow-level
permissionsinto the specific jobs that require them across multiple CI workflows. - Replace quoted
"on":withon:in select workflows. - Adjust the
run-integration-testscomposite action to passTEST_SCOPEviaenvrather than interpolating into therun:block.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/tag-recreate-lts.yml | Splits contents: write and packages: write permissions across the jobs that need them. |
| .github/workflows/sonar-pr.yml | Moves Sonar PR scan permissions from workflow to job scope. |
| .github/workflows/sonar-merge-main.yml | Moves Sonar merge-to-main permissions from workflow to job scope. |
| .github/workflows/snyk.yml | Removes workflow-level permissions; relies on job-scoped permissions. |
| .github/workflows/snyk-lts.yml | Removes workflow-level permissions; relies on job-scoped permissions. |
| .github/workflows/scorecard.yml | Replaces read-all with explicit read permissions at workflow level. |
| .github/workflows/scenario-integration-tests.yml | Moves PR/scenario integration workflow permissions to the job. |
| .github/workflows/pythonpublish.yml | Moves contents: read into the publish job permissions alongside id-token: write. |
| .github/workflows/publish.yml | Removes workflow-level contents: read (jobs already define needed permissions). |
| .github/workflows/publish-docs.yml | Removes workflow-level contents: read (deploy job uses job-scoped contents: write). |
| .github/workflows/pr-tests.yml | Moves PR test permissions to each job. |
| .github/workflows/pip-audit.yml | Moves contents: read to the job. |
| .github/workflows/nightly.yml | Moves Nightly workflow permissions down to each job / reusable workflow call. |
| .github/workflows/format.yml | Fixes on: syntax and scopes permissions to the lint job. |
| .github/workflows/codeql.yml | Fixes on: syntax and scopes contents: read to the CodeQL job. |
| .github/workflows/bdd-interop-tests.yml | Moves workflow-level permissions to the test job. |
| .github/workflows/bdd-integration-tests.yml | Partially moves permissions to the job, but retains workflow-level permission. |
| .github/actions/run-integration-tests/action.yml | Passes TEST_SCOPE via env rather than inline expression expansion in run:. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| permissions: | ||
| contents: read | ||
| pull-requests: read | ||
| checks: write |
There was a problem hiding this comment.
permissions is still defined at the workflow level (currently checks: write). This keeps elevated permissions enabled for the entire workflow and is inconsistent with the stated goal of job-scoped least-privilege. Move/remove the workflow-level permissions block and keep checks: write only on the job(s) that require it.
There was a problem hiding this comment.
This is contradicting what sonarqube flagged
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Patrick St-Louis <43082425+PatStLouis@users.noreply.github.com>
Summary
Moves GitHub Actions permissions from workflow level to job level across workflows and fixes minor workflow syntax. No change in behavior; each job still has the permissions it needs.
Changes
contents: read,pull-requests: read, andchecks: writewhere present and set the same permissions on the jobs that use them (e.g. PR Tests, Nightly, Format, BDD integration/interop, Scenario integration, Sonar PR/merge, CodeQL, pip-audit, publish-docs, pythonpublish, Snyk). Write permissions (contents: write,packages: write) remain only on the jobs that need them (e.g. Tag and Recreate LTS, Publish)."on":withon:informat.ymlandcodeql.yml.contents: readandactions: read(noread-all).TEST_SCOPEis passed via env instead of inline in the run script (addresses user-controlled data in run block).Why